-
-
Notifications
You must be signed in to change notification settings - Fork 28
[trello.com/c/4FomJzhO] Add Copy action to Failed messages menu + Open menu on long tap #833
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
…ctoring of copy mechanizm.
/run-tests |
# Conflicts: # Adamant/Modules/Chat/View/Subviews/ChatBaseMessage/ChatMessageCell.swift # Adamant/Modules/Chat/View/Subviews/ChatMedia/ChatMediaCell.swift # Adamant/Modules/Chat/View/Subviews/ChatReply/ChatMessageReplyCell.swift # Adamant/Modules/Chat/View/Subviews/ChatTransaction/Container/ChatTransactionContainerView.swift
Don’t merge to dev it because of many changes. It’s for release v3.12. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the copy functionality and long press gesture handling across multiple chat components to reduce boilerplate code while adding support for failed message interactions.
- Extracts copy-to-pasteboard behavior into shared dialog methods.
- Consolidates long press gesture handling via a common GestureHelper interface.
- Updates property names and visibility modifiers to better reflect intended usage.
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
CommonKit/Helpers/TaskManager.swift | Made TaskManager API public and updated header comments. |
CommonKit/Helpers/GestureHelper.swift | Introduced a protocol extension for processing long press gestures. |
AdamantDialogService.swift & DialogService.swift | Added and integrated a new copyToPasteboard method. |
PartnerQRViewModel.swift, ChatViewModel.swift | Updated copy actions to use the new dialogService methods. |
Various Chat module files | Refactored long press and copy gesture handling to use copyAction closures and GestureHelper. |
ChatMenuManager.swift, ChatDialogManager.swift, ChatDataSourceManager.swift, ChatAction.swift, ChatFactory.swift | Updated state management and action handling for failed message scenarios and consistent copy actions. |
Comments suppressed due to low confidence (1)
Adamant/Modules/Chat/View/Subviews/ChatTransaction/Container/ChatTransactionContainerView.swift:133
- [nitpick] Property names in Swift should follow lowerCamelCase. Consider renaming 'GestureTaskManager' to 'gestureTaskManager' for consistency with Swift style guidelines.
var GestureTaskManager: TaskManager = TaskManager()
self?.cellContainerView.animatePressDown() }, | ||
onGestureEnded: { [weak self] in self?.messageContainerView.animatePressUp() } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The onGestureEnded closure in handleLongPressToOpenMenu references 'messageContainerView', but ChatMediaCell defines 'cellContainerView'. Please update it to use the correct property for consistency.
self?.cellContainerView.animatePressDown() }, | |
onGestureEnded: { [weak self] in self?.messageContainerView.animatePressUp() } | |
self?.cellContainerView.animatePressDown() }, | |
onGestureEnded: { [weak self] in self?.cellContainerView.animatePressUp() } |
Copilot uses AI. Check for mistakes.
} | ||
|
||
extension ChatMessageCell: ChatMenuManagerDelegate { | ||
var isFailedMessage: Bool { | ||
model.backgroundColor == .failed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally speaking ,deciding on the state based on backgroundColor is not a future-proof solution. Would you consider improving this part a bit?
model.backgroundColor == .failed | ||
} | ||
|
||
func showFailedMenu(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
showFailedMenuAlert ?
While working on the task, I noticed some shortcomings that initially seemed minor. However, due to the cell system, they turned out to be significant, leading to a considerable amount of boilerplate code. I hope I managed to reduce this boilerplate by extracting the copy functionality and the animated press effect, introduced by Vladimir, into separate methods. Additionally, I aimed to improve the code by relocating functions unrelated to ChatMenuManager into the cell.